Add check that page title is in sync with ToC, h1, and metadata #3669
Conversation
|
Thanks for contributing to Qiskit documentation! Before your PR can be merged, it will first need to pass continuous integration tests and be reviewed. Sometimes the review process can be slow, so please be patient. Thanks! 🙌 |
There was a problem hiding this comment.
Thank you! This is looking really good 🎉
Aside from a few small things, there's one change I think we should make before merging this:
At the moment, both collectInvalidImageErrors and collectHeadingTitleMismatch accept a markdown string, then parse that string into an abstract syntax tree. The parsing step is relatively slow, so I think we should pull that out of the functions so we only parse the tree once.
To do this, you'd move the parsing step to L92 of checkMarkdown.ts, then change the two "collect..." functions to accept a tree: Root rather than markdown: string. Then tweak collectInvalidImageErrors so it re-uses the same tree, rather than parsing again.
| function parseMarkdown(markdown: string): Root { | ||
| return unified() | ||
| .use(remarkParse) | ||
| .use(remarkGfm) | ||
| .use(remarkFrontmatter, ["yaml"]) | ||
| .parse(markdown); | ||
| } |
There was a problem hiding this comment.
This is a nice helper function, what do you think about moving it to a new file (e.g. scripts/js/lib/markdownUtils.ts) and re-using it in checkMarkdown.ts?
There was a problem hiding this comment.
This comment still seems relevant
scripts/js/commands/checkMarkdown.ts
Outdated
| @@ -0,0 +1,147 @@ | |||
| // This code is a Qiskit project. | |||
| // | |||
| // (C) Copyright IBM 2024. | |||
There was a problem hiding this comment.
| // (C) Copyright IBM 2024. | |
| // (C) Copyright IBM 2025. |
scripts/js/commands/checkMarkdown.ts
Outdated
| type: "boolean", | ||
| default: false, | ||
| description: | ||
| "Check files in the current and dev versions of the API docs have matching titles and metadata.", |
There was a problem hiding this comment.
Now that we check both titles and images, we shouldn't be overly specific
| "Check files in the current and dev versions of the API docs have matching titles and metadata.", | |
| "Check files in the current and dev versions of the API docs.", |
| function parseMarkdown(markdown: string): Root { | ||
| return unified() | ||
| .use(remarkParse) | ||
| .use(remarkGfm) | ||
| .use(remarkFrontmatter, ["yaml"]) | ||
| .parse(markdown); | ||
| } |
There was a problem hiding this comment.
This comment still seems relevant
#2489 Added a new function and test case for mismatched title and heading